Add smithy-cfn-json build plugin#3095
Conversation
Customers deploying API Gateway REST APIs via CloudFormation need
Smithy JSON AST output with `Fn::Sub` intrinsic function wrapping so
that CFN resolves resource references before passing the body to the
SmithyImporter. Without this plugin, customers must go through the
OpenAPI conversion path to get `Fn::Sub` support.
Add a new `smithy-aws-apigateway-cfn` subproject containing the
`smithy-cfn-json` build plugin. The plugin serializes a Smithy model
to JSON AST via `ModelSerializer`, then walks the tree with a
`NodeVisitor` that wraps `${...}` strings at known trait paths in
`{"Fn::Sub": "..."}` objects. Known paths cover integration URIs,
credentials, connection IDs, integration targets, authorizer URIs
and credentials, and Cognito provider ARNs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| dependencies { | ||
| api(project(":smithy-build")) | ||
| api(project(":smithy-model")) | ||
| testImplementation(project(":smithy-aws-apigateway-traits")) |
There was a problem hiding this comment.
this is used for testing, but i guess we can use another trait for testing and remove this dependency
There was a problem hiding this comment.
If we want this plugin to work beyond API Gateway, we could test with a simple inline trait instead of depending on the apigateway-traits module. That also proves it works for any trait with ARN fields.
| @Test | ||
| public void isDiscoverableViaSpi() { | ||
| boolean found = false; | ||
| for (SmithyBuildPlugin plugin : ServiceLoader.load(SmithyBuildPlugin.class)) { |
There was a problem hiding this comment.
IIUC, when this test passes, it means we can use this plugin in smithy-build.json
| IoUtils.readUtf8Resource(getClass(), "integration-model.expected.json")) | ||
| .expectObjectNode(); | ||
|
|
||
| Node.assertEquals(actual, expected); |
There was a problem hiding this comment.
this test is a bit unnecessary since we have the integration test, but this covers the file -> file generation
…8a0ffb2b.json Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| String values containing ``${...}`` variable syntax at known trait paths are | ||
| automatically wrapped in ``{"Fn::Sub": "..."}`` objects so that CloudFormation | ||
| resolves the references at deploy time before passing the body to the API | ||
| Gateway SmithyImporter. |
There was a problem hiding this comment.
what is SmithyImporter? (˵ ¬ᴗ¬˵)
| ========================================================== | ||
| Converting Smithy to CloudFormation JSON | ||
| ========================================================== | ||
|
|
There was a problem hiding this comment.
I like this!! The NodeVisitor approach and path-matching with wildcards are clean.
One note, the core Fn::Sub injection logic is generic, it's not really API Gateway-specific. Any trait with ARN-valued fields could benefit. Would it make sense to put this in a more general module (e.g., smithy-aws-cloudformation) and make the paths configurable via the plugin config?
The API Gateway paths could be defaults, but users could add their own for custom traits. This would also avoid needing to modify the class every time we add a new trait with an ARN field (like we just did with providerARNs on authorizers)!
| private static final String SUBSTITUTION_KEY = "Fn::Sub"; | ||
| private static final Pattern SUBSTITUTION_PATTERN = Pattern.compile("\\$\\{.+}"); | ||
|
|
||
| static final List<String> PATHS = Arrays.asList( |
There was a problem hiding this comment.
These paths are hardcoded to API Gateway traits. If someone adds a new trait with an ARN field, they will have to modify this class. can we make these configurable via SmithyCfnJsonConfig? something like additionalSubstitutionPaths that users can set in smithy-build.json. The defaults could still be these API Gateway paths, but it future-proofs the plugin.
| * | ||
| * @return Returns true if substitution is disabled. | ||
| */ | ||
| public boolean getDisableCloudFormationSubstitution() { |
There was a problem hiding this comment.
isDisable... right? since its a boolean
| Model model = context.getModel(); | ||
| ServiceShape service = model.expectShape(serviceId, ServiceShape.class); | ||
|
|
||
| ObjectNode astNode = ModelSerializer.builder().build().serialize(model); |
There was a problem hiding this comment.
This serializes every shape in the model, right? and not just the ones the service uses?
Should it filter to only the service's shapes so the output doesn't include unrelated stuff?
| * ``aws.apigateway#integration`` — ``uri``, ``credentials``, ``connectionId``, | ||
| ``integrationTarget`` | ||
| * ``aws.apigateway#authorizers`` — ``*/uri``, ``*/credentials`` | ||
| * ``aws.auth#cognitoUserPools`` — ``providerArns/*`` |
There was a problem hiding this comment.
this should be apigateway cognito authorizer providerarns: #3085
I am yet to discuss with the smithy team re: aws auth's cognito user pools trait: https://smithy.io/2.0/aws/aws-auth.html#aws-auth-cognitouserpools-trait
There's a lot of confusion between these two authorizing traits.
2 participants
Background
smithy-cfn-jsonbuild plugin (in thesmithy-aws-apigateway-cfnsubproject) that serializes a Smithy model to JSON AST with CloudFormationFn::Subintrinsic function wrapping at known trait paths. The output is could be used in CFN template.Fn::Subsupport for deploy-time resource references like Lambda ARNs and VPC Link IDs. This plugin provides the same capability for the native Smithy JSON AST import path.Testing
CloudFormationFnSubInjector: verifies substitution at each known path, no substitution without${...}pattern, no substitution at non-known paths, wildcard matching, non-string node preservationSmithyCfnJson: plugin name,requiresValidModel, SPI discovery viaServiceLoaderSmithyCfnJsonConfig: deserialization with full config and defaultsFn::Subwrapping onuri,credentials,connectionId,integrationTarget; disabled mode matches rawModelSerializeroutputLinks
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.